Skip to content

[dotnet] Add .NET-specific .editorconfig #14803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 12 commits into from
Closed

[dotnet] Add .NET-specific .editorconfig #14803

wants to merge 12 commits into from

Conversation

@RenderMichael
Copy link
Contributor

@RenderMichael RenderMichael commented Nov 25, 2024 •
edited by qodo-merge-pro bot
Loading

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Specify .editorconfig settings for style references within the repo, at least for rules which are not considered "default" by the average IDE.

Motivation and Context

This will prevent suggestions for changes which are out of line with the style of the .NET code. In fact, it will enable some fixers such as IDE0009: Add this. to the code suggestions.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

enhancement, configuration changes

Description

  • Updated the .editorconfig file to align with C# style preferences.
  • Added settings to control the use of var and type qualification for various C# elements.
  • These changes aim to prevent style suggestion conflicts and enable automatic code fixers.

Changes walkthrough 📝

Relevant files
Configuration changes
.editorconfig
Update .editorconfig with C# style preferences                     

.editorconfig

  • Added C# style preferences for primary constructors and object
    creation.
  • Configured var usage rules for apparent types and built-in types.
  • Set qualification preferences for events, fields, methods, and
    properties.
    		•  +11/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @RenderMichael RenderMichael changed the title [dotnet] Align .editorconfig with C# style preferences [dotnet] Align .editorconfig with C# style preferences Nov 25, 2024
    @qodo-merge-pro qodo-merge-pro bot changed the title [dotnet] Align .editorconfig with C# style preferences [dotnet] Align .editorconfig with C# style preferences Nov 25, 2024
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Nov 25, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Style Conflict
    The combination of enabling var for apparent types while disabling it elsewhere may lead to inconsistent code style. Consider aligning these rules for better consistency.

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Nov 25, 2024 •
    edited
    Loading

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    ✅ Add missing severity level to code style rule to ensure consistent enforcement
    Suggestion Impact:The line 'csharp_style_var_elsewhere = false' was removed, which aligns with the suggestion to modify it. However, the exact change to add a severity level was not implemented.

    code diff:

    -csharp_style_prefer_primary_constructors = false
-csharp_style_implicit_object_creation_when_type_is_apparent = false:silent
-csharp_style_var_when_type_is_apparent = true:silent
-csharp_style_var_for_built_in_types = false:suggestion
-csharp_style_var_elsewhere = false

    Add severity level (e.g. :suggestion, :silent) for 'csharp_style_var_elsewhere' to
    ensure consistent enforcement across IDEs and build tools

    .editorconfig [49]

    -csharp_style_var_elsewhere = false
+csharp_style_var_elsewhere = false:suggestion
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a severity level to the style rule is important for consistent code style enforcement across different development environments and tools, aligning with how other similar rules are configured in the file.

    		7

    💡 Need additional feedback ? start a PR chat

    RenderMichael
    RenderMichael commented Nov 25, 2024
    View reviewed changes
    @nvborisenko
    Copy link
    Member

    nvborisenko commented Nov 25, 2024

    You touched untouchable. This file will be growing soon fast, I propose to create new one in dotnet space, where we will specify rules especially applied for dotnet.

    Now the question is what rules we can apply at this moment, and what rules we want to apply at the end of the story. The end of the story, I guess, when we follow rules applied by the dotnet itself (https://github.com/dotnet/runtime/blob/main/.editorconfig).

    @RenderMichael
    Copy link
    Contributor Author

    RenderMichael commented Nov 25, 2024

    @nvborisenko Sure, I'll make another file in the dotnet folder for this.

    The reason I wanted to add this is specifically for styles this project uses, which are different from the default .NET style. The biggest one is prepending every instance member with this.. The IDE has otherwise perfectly acceptable defaults.

    @RenderMichael
    Copy link
    Contributor Author

    RenderMichael commented Nov 25, 2024

    The .NET runtime has all their configuration in the newer .globalconfig format, their .editorconfig file is just for the baseline

    https://github.com/dotnet/runtime/blob/main/eng/CodeAnalysis.src.globalconfig

    @nvborisenko
    Copy link
    Member

    nvborisenko commented Nov 25, 2024

    editorconfig files inherit properties from parent folder. Ideally the editorconfig in the root git folder should have is_global = true.

    So, prepare for long discussion!

    @RenderMichael
    Copy link
    Contributor Author

    RenderMichael commented Nov 25, 2024

    I looked it up, root = true is just a hint to stop searching in base directories for more .editorconfig files. So, what we have now works. I agree it would be nice to specify that property on the root config file. but it's not necessary (at least for this PR)

    RenderMichael
    RenderMichael commented Nov 25, 2024
    View reviewed changes
    dotnet/WebDriver.NET.sln
    EndProject
    Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "WebDriver.Support.Tests", "test\support\WebDriver.Support.Tests.csproj", "{2136C695-2526-45E0-AE1D-68FBBC6A9DE2}"
    EndProject
    Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{02EA681E-C7D8-13C7-8484-4AC65E1B71E8}"
    Copy link
    Contributor Author

    @RenderMichael RenderMichael Nov 25, 2024

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    This virtual directory just makes life a little easier

    image

    @RenderMichael
    Copy link
    Contributor Author

    RenderMichael commented Nov 25, 2024

    I finished porting the .NET runtime's .editorconfig settings into our own, with the exception of the specific guidelines we use (the this. prefix)

    @RenderMichael RenderMichael changed the title [dotnet] Align .editorconfig with C# style preferences [dotnet] Add .NET-specific .editorconfig Nov 27, 2024
    @nvborisenko nvborisenko mentioned this pull request Dec 2, 2024
    Merged
    8 tasks
    @RenderMichael RenderMichael deleted the dotnet-editorconfig branch February 27, 2025 15:11
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Labels

    Review effort [1-5]: 1

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @RenderMichael @nvborisenko